Build fix - #3
Conversation
|
Project wouldn't compile on intel mac. The header file would not resolve correctly. Only by editing this file was I able to get the file to resolve the FreeTDS includes. The inclusion of the To resolve, I used the simlink location for the freetds install. However, this seems to have broken the CI pipeline you have created. To address this, without being able to re-test. I have added a compiler exception for x86 processors, where it will include the symlink but default to your configuration otherwise. I don't like this as it adds complexity. |
vkuttyp
left a comment
There was a problem hiding this comment.
Thanks for catching this — Intel Mac support is a real bug that needs fixing. The direction is right, but I'd like to address it more cleanly rather than hardcoding either path or using #if arch.
The cleanest fix is to provide both include paths via unsafeFlags in Package.swift and go back to angle-bracket includes in the header — the compiler will silently ignore whichever path doesn't exist on the current machine:
// Package.swift — CSybdb target cSettings:
cSettings: [
.unsafeFlags([
"-I/opt/homebrew/opt/freetds/include", // Apple Silicon
"-I/usr/local/opt/freetds/include", // Intel
]),
],
// CSybdb.h — back to angle brackets, no hardcoded paths:
#include <sybfront.h>
#include <sybdb.h>
This way the header works on both architectures, no #if arch needed, and it also works on Linux without changes. Could you update the PR with this approach?
…in both X86 and ARM64 locations. Added all build targets to test target as well, so the program compiles under test and build instructions.
hfoxwell
left a comment
There was a problem hiding this comment.
Thank you for your feedback, you hit the nail on the head with the added compiler flags.
I added them in and simplified the C header imports, so, now they should work across both systems.
I also added the build parameters to the testing target. This way the project will now compile under both
vkuttyp
left a comment
There was a problem hiding this comment.
Great work — this is the right fix. Clean header with angle-bracket includes, both Homebrew paths passed via -Xcc in swiftSettings, and properly gated with .when(platforms: [.macOS]). This will work correctly on both Apple Silicon and Intel without any hardcoded assumptions.
Minor suggestion for a follow-up: the cSettings/swiftSettings/linkerSettings blocks on the test target are duplicates of what the main target already provides — the test target inherits these through its dependency on SQLClientSwift. Feel free to strip those from the test target to keep Package.swift cleaner.
Approving — please make sure CI passes before we merge.
Changes
Sources/CFreeTDS/include/CFreeTDS.hReplaced hardcoded Cellar paths (which broke on every
brew upgrade) with Homebrew's stable opt symlink. Linux continues to use system resolution via angle bracket includes./usr/local/opt/freetds/include/on macOS — a stable symlink maintained by Homebrew regardless of installed versionpkg-configPackage.swiftUpdated linker settings to support both Intel and Apple Silicon Macs by providing both Homebrew prefix locations. The compiler silently ignores paths that don't exist, so providing both is safe.
-L/opt/homebrew/opt/freetds/libfor Apple Silicon-L/usr/local/opt/freetds/libfor IntelpkgConfig: "freetds"and theaptprovider on thesystemLibrarytargetBuild prerequisites (macOS):
This now allows for the project to compile and build on my intel mac as well as ARM Macs